[Clang][OpenCL] Fix wait_for_event argument address space with -fdeclare-opencl-builtins #134598
+24
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…cl-builtins The pointer argument of the wait_for_event builtin should take the default address space: generic if available, otherwise private.
jmmartinez
changed the title
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: Juan Manuel Martinez Caamaño (jmmartinez) ChangesThe pointer argument for Before this patch it would always be generic with Full diff: https://github.com/llvm/llvm-project/pull/134598.diff 2 Files Affected:
diff --git a/clang/lib/Sema/OpenCLBuiltins.td b/clang/lib/Sema/OpenCLBuiltins.td
index 4da61429fcce7..528b700a275e0 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -958,13 +958,25 @@ foreach name = ["async_work_group_strided_copy"] in {
def : Builtin<name, [Event, PointerType<AGenTypeN, LocalAS>, PointerType<ConstType<AGenTypeN>, GlobalAS>, Size, Size, Event]>;
def : Builtin<name, [Event, PointerType<AGenTypeN, GlobalAS>, PointerType<ConstType<AGenTypeN>, LocalAS>, Size, Size, Event]>;
}
-foreach name = ["wait_group_events"] in {
- def : Builtin<name, [Void, Int, PointerType<Event, GenericAS>]>;
-}
foreach name = ["prefetch"] in {
def : Builtin<name, [Void, PointerType<ConstType<AGenTypeN>, GlobalAS>, Size]>;
}
+// The wait_group_events is declared with an argument of type event_t*.
+// The address-space of the pointer parameter is different if the generic address space is available.
+multiclass BuiltinWithDefaultPointerArg<AddressSpace AS> {
+ foreach name = ["wait_group_events"] in {
+ def : Builtin<name, [Void, Int, PointerType<Event, AS>]>;
+ }
+}
+
+let Extension = FuncExtOpenCLCNamedAddressSpaceBuiltins in {
+ defm : BuiltinWithDefaultPointerArg<PrivateAS>;
+}
+let Extension = FuncExtOpenCLCGenericAddressSpace in {
+ defm : BuiltinWithDefaultPointerArg<GenericAS>;
+}
+
//--------------------------------------------------------------------
// OpenCL v2.0 s6.13.11 - Atomics Functions.
// Functions that use memory_order and cl_mem_fence_flags enums are not
diff --git a/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl b/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
index ac3bff9dbde27..8bd1db5d06819 100644
--- a/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
+++ b/clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
@@ -48,6 +48,15 @@ void test_generic_optionality(float a, float *b) {
float res = fract(a, b);
}
+// Test that the correct builtin is called depending on the generic address
+// space feature availability. If not available, the __private version is called
+// CHECK-LABEL: @test_wait_group_events
+// CHECK-GAS: call spir_func void @_Z17wait_group_eventsiPU3AS49ocl_event
+// CHECK-NOGAS: call spir_func void @_Z17wait_group_eventsiP9ocl_event
+void test_wait_group_events(int i, event_t *e) {
+ wait_group_events(i, e);
+}
+
// CHECK: attributes [[ATTR_CONST]] =
// CHECK-SAME: memory(none)
// CHECK: attributes [[ATTR_PURE]] =
|
Contributor
Author
|
Thanks for the quick review ! |
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Apr 24, 2025
-fdeclare-opencl-builtins Comgr does not embed anymore the precompiled versions of opencl-c, however, it still has to embed the source version of opencl-c-base (on some builds we do not ship Clang's resource-dir). The AMD_COMGR_ACTION_ADD_PRECOMPILED_HEADERS is kept around for compatibility, but it doesn't add the headers anymore to the output set. If used, the input set is forwarded to the output set unchanged. Depens on llvm#134598 to pass a full OpenCL Conformance test.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The pointer argument for
wait_for_event(int, event_t*)should take the default address space: generic if available, otherwise private.Before this patch it would always be generic with
-fdeclare-opencl-builtins. This was inconsistent with the behavior when opencl-c.h is included.